chore: Update React Native to 0.73.2 #31558

j-piasecki · 2023-11-20T16:46:02Z

Details

PR bumping RN version of the App to 0.73.2.

Some types have changed in the RN core and/or Android SDK 34, so we had to add patches for libraries to make them compile on Android (all of those are related to nullability on Kotlin).

Changed patches:

@onfido/react-native-sdk - due to upgraded AGP, publishing components are no longer created automatically and need to be defined manually (see https://developer.android.com/build/releases/past-releases/agp-7-1-0-release-notes#disable-component-creation), upstream PR: Add publishing block in build.gradle onfido/react-native-sdk#157

Added patches:

react-native-vision-camera due to changed nullability type in SDK 34 the library wouldn't compile, there's already a motion to use Vision Camera 3, where this problem should be fixed

Removed patches:

patch that was removing usage of setState from KeyboardAvoidingView - those changes are included in RN 0.73 - Don't use setState for disabled KeyboardAvoidingView to avoid re-renders facebook/react-native#38074

Important notes:

SelectionList perf test was modified due to a change in VirtualList implementation (Fix virtualization logic with horizontal RTL lists facebook/react-native#38529) it's also necessary for now to invoke onContentSizeChange event as it's no longer handled in onScroll handler. It's possible that migrating to the userEvent API for events will make it redundant in the future (fireEvent.scroll doesn't update the FlatList's render window on React Native 0.73 callstack/react-native-testing-library#1540)
in AGP 8.0 some default values have changed, most notably nonTransitiveRClass, which caused build issues with react-native-screens and @rnmapbox/maps as they use R values from outside the packages
npx expo install --fix was run to make sure relevant dependencies are compatible with Expo SDK 50

Fixed Issues

$ #31381
$ #22155
PROPOSAL:

Tests

Verify that no errors appear in the JS console

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

melvin-bot · 2023-11-20T16:46:42Z

@jasperhuangg Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

situchan · 2023-11-21T08:59:27Z

All checks are failing. Please merge main and see if still fails

situchan · 2024-01-11T10:44:26Z

@github-actions
Reassure Performance Tests / perf-tests (pull_request) Failing after 22m

Is this known?

WoLewicki · 2024-01-11T10:45:57Z

Yes, locally they pass so maybe some problems with some CI caches.

situchan · 2024-01-11T12:17:02Z

ios/Podfile.lock

-  YogaKit: f782866e155069a2cca2517aafea43200b01fd5a
+  Turf: 13d1a92d969ca0311bbc26e8356cca178ce95da2
+  VisionCamera: 7d13aae043ffb38b224a0f725d1e23ca9c190fe7
+  Yoga: e64aa65de36c0832d04e8c7bd614396c77a80047


This checksum is different in my local

It passes on CI so should we care about it? I personally do not have changes in it so not sure what is the cause.

No idea why is that, I've cleared npm cache, derived data, reinstalled node modules and pods, and it stays the same for me.

situchan · 2024-01-11T12:33:22Z

I am getting this error locally

j-piasecki · 2024-01-11T12:39:42Z

@situchan It looks like you're using Android Studio Flamingo, which doesn't support that version of AGP (see here). If that's not it, then I'm not sure what could be the cause.

situchan · 2024-01-11T12:54:46Z

@situchan It looks like you're using Android Studio Flamingo, which doesn't support that version of AGP (see here). If that's not it, then I'm not sure what could be the cause.

correct. now updating to Hedgehog

situchan · 2024-01-11T16:19:15Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

mountiny

Tests are passing and @situchan @shubham1206agra could not find any other issues, I think this would be great to merge and deploy to staging before the weekend if possible @roryabraham

OSBotify · 2024-01-11T17:51:36Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

situchan · 2024-01-15T06:21:03Z

I think this came from RN upgrade.
Offline message briefly shows after refresh website.

Screen.Recording.2024-01-15.at.12.17.22.PM.mov

@j-piasecki @WoLewicki can you please check?

situchan · 2024-01-15T06:26:48Z

package.json

    "@react-native-camera-roll/camera-roll": "5.4.0",
    "@react-native-clipboard/clipboard": "^1.12.1",
    "@react-native-community/geolocation": "^3.0.6",
-    "@react-native-community/netinfo": "^9.3.10",
+    "@react-native-community/netinfo": "11.1.0",


Confirmed this is the culprit. Not happen on 9.3.10

Seems fixed in latest version (11.2.1)
react-native-netinfo/react-native-netinfo#706 is the fixing PR

Should we submit a quick PR with the bump of version or do you want to do it?

can you submit if you can? I will handle review

Ok, on it now 🚀

here it is: #34491

OSBotify · 2024-01-15T11:56:38Z

🚀 Deployed to staging by https://github.com/roryabraham in version: 1.4.25-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

kidroca · 2024-01-15T13:41:28Z

package.json

    "onchange": "^7.1.0",
    "portfinder": "^1.0.28",
    "prettier": "^2.8.8",
    "pusher-js-mock": "^0.3.3",
    "react-native-clean-project": "^4.0.0-alpha4.0",
-    "react-native-flipper": "https://gitpkg.now.sh/facebook/flipper/react-native/react-native-flipper?9cacc9b59402550eae866e0e81e5f0c2f8203e6b",


It seems the flipper plugin used for debugging was removed. There's also this warning during development

Perhaps this was a mistake because Flipper is still being used in the project

Could you please clarify if the removal of the Flipper plugin was intentional and, if so, provide the reasoning behind this decision?

This was dicussed here, not a blocker, Flipper will be removed https://expensify.slack.com/archives/C05LX9D6E07/p1705066075250729?thread_ts=1704921323.369379&cid=C05LX9D6E07

shubham1206agra · 2024-01-15T16:18:23Z

package.json

-    "react-native-pager-view": "^6.2.0",
-    "react-native-pdf": "^6.7.3",
+    "react-native-pager-view": "6.2.2",
+    "react-native-pdf": "^6.7.4",


Whoops. This has caused #34510 which @situchan previously solved by downgrading version.

@situchan Can you downgrade this again?

Weird. Seems like merge conflict.
Gitlens also showing old PR

Then revert it again to fix this 😄

OSBotify · 2024-01-17T01:14:22Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.4.25-10 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

situchan · 2024-01-31T04:34:18Z

android/build.gradle

-        // We use NDK 23 which has both M1 support and is the side-by-side NDK version from AGP.
-        ndkVersion = "23.1.7779620"
+        compileSdkVersion = 34
+        targetSdkVersion = 34


Bumping targetSdkVersion to 34 caused crash on android 14 devices.
The root cause was in react-native-blob-utils package, which we had to bump version to fix it.

j-piasecki added 4 commits November 20, 2023 12:15

WIP: Android builds

c72c5cd

WIP: ios

5377269

Add patches

c95f25a

Update onfido patch

c61983e

j-piasecki requested a review from a team as a code owner November 20, 2023 16:46

melvin-bot bot requested review from jasperhuangg and removed request for a team November 20, 2023 16:46

Uncomment flavors

58b5b85

fabioh8010 mentioned this pull request Nov 20, 2023

[$500] Do not allow emojis to be formatted (ie italics) #14676

Open

3 tasks

WoLewicki mentioned this pull request Nov 21, 2023

[HOLD for payment 2024-01-24] [$500] Upgrade react-native to 0.73+ #31381

Closed

j-piasecki added 8 commits November 21, 2023 10:42

Update onfido patch

41b6807

Merge branch 'main' into @jpiasecki/rn-73

266025a

Update to rc.5

4738532

Restore deleted manifests

6305f1c

Restore newlines

fe9d82e

Update rngh

bd505cc

Update patches

05e22df

Convert MainActivity and MainApplication to kotlin

c3ea355

arosiclair mentioned this pull request Nov 27, 2023

[$1000] Chat - Emojis in bold and italic style appear smaller in size #15424

Closed

6 tasks

j-piasecki and others added 2 commits November 28, 2023 10:06

Use overrides for RN and Reanimated versions

a4d30b0

chore: bump rn and reanimated

5f664e6

j-piasecki changed the title ~~chore: Update React Native to 0.73.0-rc.4~~ chore: Update React Native to 0.73.0-rc.6 Nov 28, 2023

chore: merge current main

e4cae6d

AndrewGable added the Ready To Build label Nov 28, 2023

This comment has been minimized.

Sign in to view

muttmuure mentioned this pull request Nov 29, 2023

[CRITICAL] Upgrade to Fabric #29258

Closed

This comment was marked as outdated.

Sign in to view

j-piasecki added 2 commits January 11, 2024 12:35

Merge branch 'main' into @jpiasecki/rn-73

3b605de

Remove upstreamed patch

12adc94

situchan reviewed Jan 11, 2024

View reviewed changes

j-piasecki added 2 commits January 11, 2024 17:06

Merge branch 'main' into @jpiasecki/rn-73

75de5f2

Fix test not working after revert

40b4efe

mountiny approved these changes Jan 11, 2024

View reviewed changes

roryabraham approved these changes Jan 11, 2024

View reviewed changes

roryabraham merged commit d541723 into Expensify:main Jan 11, 2024
17 of 19 checks passed

danieldoglas mentioned this pull request Jan 11, 2024

TextInput numberOfLines/maxNumberOfLines workaround (manually calculating maxHeight of TextInput #31483

Merged

50 tasks

AndrewGable mentioned this pull request Jan 11, 2024

[NoQA] Use --mode instead of --configuration to fix npm run ios #34399

Merged

48 tasks

hannojg mentioned this pull request Jan 12, 2024

[HOLD for payment 2024-01-31] predeploy.yml Github Action is broken in main #34108

Closed

situchan reviewed Jan 15, 2024

View reviewed changes

WoLewicki mentioned this pull request Jan 15, 2024

[CP Staging] chore: bump netinfo to newest version #34491

Merged

50 tasks

github-actions bot mentioned this pull request Jan 15, 2024

Deploy Checklist: New Expensify 2024-01-15 #34493

Closed

83 tasks

kidroca reviewed Jan 15, 2024

View reviewed changes

shubham1206agra reviewed Jan 15, 2024

View reviewed changes

situchan mentioned this pull request Jan 15, 2024

[CP Staging] re-downgrade react-native-pdf version #34513

Merged

60 tasks

melvin-bot bot mentioned this pull request Jan 17, 2024

[HOLD for payment 2024-01-24] Remove RN patch once upstream PR38074 is merged #22155

Closed

situchan reviewed Jan 31, 2024

View reviewed changes

situchan mentioned this pull request Jan 31, 2024

[HOLD for payment 2024-01-30] [HOLD for payment 2024-01-25] [HOLD for payment 2024-01-24] [$500] Android - App crashes when downloading attachments #34569

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Update React Native to 0.73.2 #31558

chore: Update React Native to 0.73.2 #31558

j-piasecki commented Nov 20, 2023 •

edited

Loading

melvin-bot bot commented Nov 20, 2023

situchan commented Nov 21, 2023

This comment has been minimized.

This comment was marked as outdated.

This comment was marked as outdated.

situchan commented Jan 11, 2024

WoLewicki commented Jan 11, 2024

situchan Jan 11, 2024

WoLewicki Jan 11, 2024

j-piasecki Jan 11, 2024

situchan commented Jan 11, 2024

j-piasecki commented Jan 11, 2024

situchan commented Jan 11, 2024

situchan commented Jan 11, 2024

mountiny left a comment

OSBotify commented Jan 11, 2024

situchan commented Jan 15, 2024

situchan Jan 15, 2024

situchan Jan 15, 2024

WoLewicki Jan 15, 2024

situchan Jan 15, 2024

WoLewicki Jan 15, 2024

WoLewicki Jan 15, 2024

OSBotify commented Jan 15, 2024

kidroca Jan 15, 2024

mountiny Jan 15, 2024

shubham1206agra Jan 15, 2024

situchan Jan 15, 2024

shubham1206agra Jan 15, 2024

OSBotify commented Jan 17, 2024

situchan Jan 31, 2024

chore: Update React Native to 0.73.2 #31558

chore: Update React Native to 0.73.2 #31558

Conversation

j-piasecki commented Nov 20, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Nov 20, 2023

situchan commented Nov 21, 2023

This comment has been minimized.

This comment was marked as outdated.

This comment was marked as outdated.

situchan commented Jan 11, 2024

WoLewicki commented Jan 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

situchan commented Jan 11, 2024

j-piasecki commented Jan 11, 2024

situchan commented Jan 11, 2024

situchan commented Jan 11, 2024

Reviewer Checklist

Screenshots/Videos

mountiny left a comment

Choose a reason for hiding this comment

OSBotify commented Jan 11, 2024

situchan commented Jan 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OSBotify commented Jan 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OSBotify commented Jan 17, 2024

Choose a reason for hiding this comment

j-piasecki commented Nov 20, 2023 •

edited

Loading